-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ci: Add GH workflow for publishing to npm. #43
Conversation
Warning Rate limit exceeded@junhaoliao has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 17 minutes and 54 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis pull request introduces a new GitHub Actions workflow file Changes
Sequence DiagramsequenceDiagram
participant Release as Release Event
participant Workflow as GitHub Actions
participant Build as Build Job
participant Publish as Publish NPM Job
participant Branch as Create Branch Job
Release->>Workflow: Trigger workflow
Workflow->>Build: Execute build steps
Build-->>Publish: Upload artifacts
Publish->>Publish: Publish to NPM
Publish-->>Branch: Trigger branch creation
Branch->>Branch: Create and push new branch
Possibly Related PRs
Suggested Reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
95c6ed1
to
4b7a2be
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (4)
package.json (1)
16-16
: Consider enhancing the release script with additional safeguardsThe current release script could benefit from additional checks to ensure reliable publishing:
- "release": "git diff --exit-code && task package && npm publish" + "release": "npm version check && git diff --exit-code HEAD && task package && npm pack --dry-run && npm publish"This modification:
- Validates the package version
- Ensures clean working directory (comparing with HEAD)
- Performs a dry-run package check before publishing
.github/workflows/publish-npm.yml (3)
18-22
: Implement caching for task dependenciesAdd caching for the task CLI to improve workflow performance.
+ - uses: actions/cache@v3 + with: + path: ~/.npm + key: ${{ runner.os }}-task-${{ hashFiles('**/package-lock.json') }} + restore-keys: | + ${{ runner.os }}-task- + - name: "Install task" run: "npm install -g @go-task/cli"
61-64
: Add error handling for branch creationThe branch creation step should handle potential failures gracefully.
- name: "Create and push branch" run: | + if git show-ref --quiet refs/heads/"${{github.ref_name}}"; then + echo "Branch already exists, skipping creation" + exit 0 + fi git checkout -b "${{github.ref_name}}" git push origin "refs/heads/${{github.ref_name}}"
3-6
: Consider adding version tag filteringThe workflow triggers on all releases. Consider filtering for specific version patterns.
on: release: types: ["created"] + tags: + - 'v[0-9]+.[0-9]+.[0-9]+'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/publish-npm.yml
(1 hunks)package.json
(1 hunks)
🧰 Additional context used
🪛 GitHub Check: lint (macos-latest)
.github/workflows/publish-npm.yml
[failure] 50-50:
50:17 [quoted-strings] string value is not quoted with double quotes
🔇 Additional comments (1)
.github/workflows/publish-npm.yml (1)
8-8
: Define explicit permissions for workflow jobs
The root-level permissions block is empty. Consider defining explicit permissions to follow the principle of least privilege.
[security]
-permissions: {}
+permissions:
+ contents: read
+ packages: read
.github/workflows/publish-npm.yml
Outdated
- uses: "actions/setup-node@v4" | ||
with: | ||
node-version: 22 | ||
registry-url: "https://registry.npmjs.org/" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider using LTS version of Node.js
Node.js 22 is currently in development. For production publishing workflows, it's recommended to use an LTS version.
with:
- node-version: 22
+ node-version: 20
registry-url: "https://registry.npmjs.org/"
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
- uses: "actions/setup-node@v4" | |
with: | |
node-version: 22 | |
registry-url: "https://registry.npmjs.org/" | |
- uses: "actions/setup-node@v4" | |
with: | |
node-version: 20 | |
registry-url: "https://registry.npmjs.org/" |
.github/workflows/publish-npm.yml
Outdated
- run: "npm publish" | ||
env: | ||
NODE_AUTH_TOKEN: "${{secrets.NPM_TOKEN}}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add package validation steps before publishing
Consider adding validation steps before publishing to ensure package integrity.
+ - name: "Validate package"
+ run: |
+ npm pack --dry-run
+ npm audit
+
- run: "npm publish"
env:
NODE_AUTH_TOKEN: "${{secrets.NPM_TOKEN}}"
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
- run: "npm publish" | |
env: | |
NODE_AUTH_TOKEN: "${{secrets.NPM_TOKEN}}" | |
- name: "Validate package" | |
run: | | |
npm pack --dry-run | |
npm audit | |
- run: "npm publish" | |
env: | |
NODE_AUTH_TOKEN: "${{secrets.NPM_TOKEN}}" |
.github/workflows/publish-npm.yml
Outdated
- name: "Install task" | ||
run: "npm install -g @go-task/cli" | ||
|
||
- name: "Run package task" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if the purpose of the name is to add comments, that just repeat the command. right, we don't need it
.github/workflows/publish-npm.yml
Outdated
with: | ||
path: "dist/" | ||
|
||
publish-npm: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about publish-to-npm
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. Shall we also update the workflow name
and filename to match this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, good idea.
…rom code review Co-authored-by: kirkrodrigues <[email protected]>
…y suggestions from code review Co-authored-by: kirkrodrigues <[email protected]>
Description
Changes made
Related: #44
package.json
scriptrelease
which is used for manual releases.secrets.NPM_TOKEN
into y-scope/clp-ffi-js. @kirkrodrigues steps documented at# Validation performed -> 2.
.Making a release
With this automation workflow added, here's a proposed flow for making a release:
npm version <major | minor | patch> -m "chore: Upgrade version for %s release."
vA.B.C
; then click “Create new tag:vA.B.C
on publish”.npm install clp-ffi-js
) in a project. If good, edit the GitHub pre-release to uncheck “Set as a pre-release” and “Update release”.Validation performed
npm run release
and observednpm package
was run although the publish failed because the npm registry does not support overwriting an already published version:Token name
:GITHUB_CLP_FFI_JS_NPM_TOKEN
.Packages and scopes
->Permissions
->Read and write
->Only select packages and scopes
->Select packages and scopes
->clp-ffi-js
.NPM_TOKEN
with the generated npm token at https://github.com/junhaoliao/clp-ffi-js/settings/secrets/actions/newnpm publish
command was run although the publish failed because versionv0.3.1
already exists.npm publish
job), a branch was created with name as the version number.Summary by CodeRabbit
Summary by CodeRabbit